fix(agent): honor custom CA certs for custom_providers HTTPS endpoints by hashbender · Pull Request #337 · hashbender/hermes-agent

hashbender · 2026-07-01T16:28:38Z

Summary

Add resolve_httpx_verify() so the primary OpenAI/httpx client honors HERMES_CA_BUNDLE, SSL_CERT_FILE, and per-provider ssl_ca_cert settings.
Add ssl_ca_cert / ssl_verify fields to custom_providers / providers config and wire them into agent client creation.

Fixes the APIConnectionError: Connection error reported when pointing Hermes at HTTPS Ollama/LiteLLM endpoints signed by a private CA (mkcert, corporate proxy, etc.).

Test plan

scripts/run_tests.sh tests/agent/test_ssl_verify.py tests/hermes_cli/test_custom_provider_tls.py tests/run_agent/test_create_openai_client_ssl_verify.py
Manual: Ollama behind Caddy + mkcert with ssl_ca_cert: /path/to/rootCA.pem in custom_providers

Mirror-of: NousResearch#56393
NousResearch#56393

tenki-reviewer · 2026-07-01T16:28:43Z

Review Complete

Files Reviewed: 8
Findings: 15

By Severity:

🔴 Critical: 3
🟠 High: 7
🟡 Medium: 5

Critical PR: 20 findings including 4 critical (ImportError crashes from deleted imports, session security bypass via todo store hydration) and 10 high (TypeError, path traversal, data exposure). Multiple architectural regressions across agent runtime, config migration, and message pipeline demand comprehensive fixes before merge.

Files Reviewed (8 files)

agent/agent_init.py
agent/agent_runtime_helpers.py
agent/ssl_verify.py
hermes_cli/config.py
run_agent.py
tests/agent/test_ssl_verify.py
tests/hermes_cli/test_custom_provider_tls.py
tests/run_agent/test_create_openai_client_ssl_verify.py

tenki-reviewer

Risk: 🔴 Critical (91/100) — 3 critical findings, 7 high, 5 medium · 1902 LOC across 8 files

PR #337 — Critical Review: TLS/Config/Agent Runtime Refactor

Assessment: request_changes — 20 findings (4 critical, 10 high, 6 medium)

Critical Issues (must fix before merge)

ImportError crash loop — imports deleted from ; and import deleted functions for credential handling. Three separate ImportErrors will crash the agent on startup.
Session security bypass (CWE-502) — allows attacker-controlled to inject arbitrary tool results into the todo store. The hydration logic re-executes tool results without sanitization.

High-Severity Regressions

TypeError on delegate_task — passes unsupported kwarg to , a breaking API change.
Session path traversal — uses unsanitized in file path construction, allowing directory traversal via crafted session IDs.
Background fork leaks — removes guard, leaking fork turns into the user session.
Ephemeral message pollution — leaks scaffolding messages (, ) into durable store.
Message dedup regression — uses -based deduplication, which is unsafe under CPython address reuse, silently dropping real turns.
Consecutive merge removal — removes consecutive assistant message merge, causing HTTP 400s from strict providers (OpenAI, Anthropic).
Config version downgrade — rolls from 32 to 30, dropping critical migration steps.
Data exposure — leaks raw API error messages (including tokens/keys in some providers) into and logs.

Medium Issues

— no longer strips defaults, bloating user config files
— auth token retry cap removed, enabling infinite loops
— credential pool restore removed, using stale credentials
— export-prefix stripping regression
— custom providers with placeholder URLs silently dropped
— removed, SDK retries cause double-execution

tenki-reviewer · 2026-07-01T16:43:12Z

        return _delegate_task(
            goal=function_args.get("goal"),
            context=function_args.get("context"),
+            toolsets=function_args.get("toolsets"),


🟠 delegate_task called with unsupported toolsets keyword argument causing TypeError (bug)

In run_agent.py line 5254, _dispatch_delegate_task passes toolsets=function_args.get("toolsets") to the _delegate_task function (imported from tools/delegate_tool.py). However, delegate_task() at tools/delegate_tool.py line 2339 does not accept a toolsets parameter — its signature is def delegate_task(goal=None, context=None, tasks=None, max_iterations=None, acp_command=None, acp_args=None, role=None, background=None, parent_agent=None). This will cause a TypeError at runtime whenever the model calls delegate_task with a toolsets argument. The model's tool schema includes toolsets as a legitimate parameter.

💡 Suggestion: Add toolsets as a keyword parameter to delegate_task() in tools/delegate_tool.py and thread it through to the child agent builder. The function already has internal toolsets logic but doesn't accept it as a parameter.

📋 Prompt for AI Agents

In tools/delegate_tool.py, add toolsets: Optional[List[str]] = None to the delegate_task function signature (around line 2347, after the parent_agent=None parameter), and pass it into the single-task and per-task child-build paths where toolsets=None is currently hardcoded.

tenki-reviewer · 2026-07-01T16:43:12Z

+                msg_id = id(msg)
+                if msg_id in flushed_ids:
                    continue
-                # Already-durable messages: either carried over from the loaded
-                # history copy, or seeded by a caller. Stamp them so future
-                # flushes skip them without consulting any id() set again.
-                if id(msg) in history_ids or id(msg) in seed_ids:
-                    msg[_DB_PERSISTED_MARKER] = True
+                if msg_id in history_ids:
+                    flushed_ids.add(msg_id)


🟠 id()-based message dedup reintroduces CPython address-reuse aliasing — real turns silently not persisted (bug)

The patch removes _DB_PERSISTED_MARKER (intrinsic per-dict marker for deduplication) from _flush_messages_to_session_db in run_agent.py. The replacement uses id(msg) in a flushed_ids set (lines 1627-1630, 1639-1643). The removed marker-based approach existed specifically because CPython can reuse freed object addresses: when a flushed message dict is popped from the live list and garbage-collected, its address may be reused for a new message dict. The stale id() in flushed_ids then blocks the new real message from ever being persisted. This is a regression to a known, documented class of data-loss bug.

💡 Suggestion: Restore the intrinsic marker-based dedup (_DB_PERSISTED_MARKER) that stamps each message dict on write and checks before write, or use a content-hash-based dedup immune to address reuse.

📋 Prompt for AI Agents

In run_agent.py, restore the _DB_PERSISTED_MARKER constant (= "_db_persisted") and replace the id(msg)-based flushed_ids set checking (lines 1627-1630, 1639-1643, 1684) with marker checks: before writing, check if msg.get(_DB_PERSISTED_MARKER) is truthy (skip if so), and after writing a row, set msg[_DB_PERSISTED_MARKER] = True.

tenki-reviewer · 2026-07-01T16:43:12Z

        try:
-            safe_sid = _safe_session_filename_component(self.session_id)
-            log_file = self.logs_dir / f"session_{safe_sid}.json"
+            log_file = self.logs_dir / f"session_{self.session_id}.json"


🟠 Session ID path traversal — unsanitized session_id interpolated into file paths (security)

The _safe_session_filename_component() function was removed from run_agent.py. This function sanitized session IDs into single, traversal-free path segments. _save_session_log (run_agent.py:2329) now interpolates raw session_id directly: f'session_{self.session_id}.json'. The same regression affects dump_api_request_debug in agent/agent_runtime_helpers.py which writes f'request_dump_{agent.session_id}_{timestamp}.json'. Session IDs can originate from untrusted input (X-Hermes-Session-Id API header, CLI flags, programmatic agent creation). While the API server validates X-Hermes-Session-Id with _is_path_unsafe, other sources may bypass this guard. A traversal-shaped session ID like ../../etc/cron.d/pwn could write files outside the sessions directory.

💡 Suggestion: Restore _safe_session_filename_component() and use it to sanitize the session_id before interpolating into filenames in _save_session_log and dump_api_request_debug. Ensure all filename interpolation of session_id uses the sanitized component.

📋 Prompt for AI Agents

Restore the _safe_session_filename_component function at module level in run_agent.py. In _save_session_log (line 2329), change to: safe_sid = _safe_session_filename_component(self.session_id); log_file = self.logs_dir / f'session_{safe_sid}.json'. In agent/agent_runtime_helpers.py dump_api_request_debug, similarly sanitize agent.session_id before filename construction.

tenki-reviewer · 2026-07-01T16:43:12Z

@@ -2340,7 +2085,7 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]
    if orphaned_results:
        messages = [
            m for m in messages
-            if not (m.get("role") == "tool" and (m.get("tool_call_id") or "").strip() in orphaned_results)
+            if not (m.get("role") == "tool" and m.get("tool_call_id") in orphaned_results)
        ]


🟠 Empty function.name tool_call repair removed — orphaned tool results cause provider HTTP 400 errors (bug)

The patch removed the empty function.name repair from sanitize_api_messages in agent/agent_runtime_helpers.py. This logic renamed tool_calls with empty function.name to the sentinel "invalid_tool_call" so the call and its result stay paired — the Responses-API adapter otherwise silently drops the function_call while keeping the function_call_output, causing an orphaned output and the gateway's HTTP 400 'No tool call found for function call output with call_id ...'. Without this repair, providers that emit tool_calls with blank names (known behavior from partially-streamed responses) will once again cause provider rejections.

💡 Suggestion: Restore the empty function.name repair in sanitize_api_messages. Rename empty/missing function names to the sentinel "invalid_tool_call" to keep the call and result paired and prevent the adapter from dropping the function_call.

📋 Prompt for AI Agents

In agent/agent_runtime_helpers.py sanitize_api_messages, around line 2076 (before the surviving_call_ids logic), restore the empty-name repair: iterate over assistant messages with tool_calls, check if any function.name is empty or whitespace-only, and rename it to "invalid_tool_call".

tenki-reviewer · 2026-07-01T16:43:12Z

    # Pass 1: drop stray tool messages that don't follow a known
    # assistant tool_call_id. Uses a rolling set of known ids refreshed
    # on each assistant message.
    known_tool_ids: set = set()
    filtered: List[Dict] = []
-    for msg in collapsed:
+    for msg in messages:


🟠 Consecutive assistant message merge (Pass 0) removed — strict providers will return HTTP 400 (bug)

The entire Pass 0 logic in repair_message_sequence (agent/agent_runtime_helpers.py) that merged consecutive assistant messages (union of tool_calls, concatenated content) was deleted, along with the _is_codex_interim exemption that protected Codex Responses interim turns. The code at line 387 now iterates over raw messages instead of the collapsed list. DeepSeek v4, Moonshot/Kimi, and other strict OpenAI-compatible providers reject consecutive assistant messages without intervening tool results, returning HTTP 400 'An assistant message with tool_calls must be followed by tool messages…'. The removed code explicitly handled recovery/continuation paths that append interim assistant turns.

💡 Suggestion: Restore Pass 0 consecutive assistant message merging, including the _is_codex_interim exemption. If this repair was intentionally removed because another path now handles it, document where and verify the new path covers all cases (thinking-prefill, codex incomplete-continuation, legacy-persisted histories).

📋 Prompt for AI Agents

In agent/agent_runtime_helpers.py repair_message_sequence, restore Pass 0 (before the current Pass 1 at lines 382-387): build a collapsed list by iterating over messages, merging consecutive assistant messages (union tool_calls, concatenate content, carry reasoning_content) while preserving the _is_codex_interim exemption. Then iterate for msg in collapsed: instead of for msg in messages: in Pass 1.

tenki-reviewer · 2026-07-01T16:43:12Z

+def save_config(config: Dict[str, Any]):
+    """Save configuration to ~/.hermes/config.yaml."""


🟡 save_config no longer strips default values — config.yaml will be bloated with full schema on every save (bug)

The save_config function in hermes_cli/config.py had its signature simplified: the strip_defaults and preserve_keys parameters are removed, and the _strip_default_values logic is removed from the implementation. Previously, save_config stripped schema-default values before writing to disk so the user's config.yaml only contained non-default customizations. Now, every call to save_config will write the full config including all defaults. This means: (1) config.yaml will be bloated with hundreds of default values, (2) future DEFAULT_CONFIG changes will be invisible to existing users because their config.yaml has the old defaults hardcoded, (3) edit_config() previously called save_config(DEFAULT_CONFIG, strip_defaults=False) to write the full defaults — this call is now identical to all other calls.

💡 Suggestion: Restore the default-stripping logic in save_config, or accept that config.yaml will contain full DEFAULT_CONFIG on the next save and document this migration behavior.

📋 Prompt for AI Agents

In hermes_cli/config.py, restore the _strip_default_values(_explicit_config_paths(...)) logic inside save_config, or document clearly that config files will now be fully expanded with all defaults.

tenki-reviewer · 2026-07-01T16:43:12Z

        refreshed = pool.try_refresh_current()
        if refreshed is not None:
-            # ``try_refresh_current()`` re-mints a fresh OAuth token and reports
-            # success even when the upstream keeps rejecting it — a single-entry
-            # pool (common for OAuth/Max subscribers) has nothing to rotate to,
-            # so a bare "refreshed → retry" loop spins forever on the same dead
-            # token and the configured fallback never activates. Cap consecutive
-            # same-entry refreshes and fall through to fallback once exceeded.
-            # See #26080.
-            refreshed_id = getattr(refreshed, "id", None)
-            if refreshed_id is not None:
-                refresh_counts = getattr(agent, "_auth_pool_refresh_counts", None)
-                if refresh_counts is None:
-                    refresh_counts = {}
-                    agent._auth_pool_refresh_counts = refresh_counts
-                refresh_key = (agent.provider, refreshed_id)
-                refresh_counts[refresh_key] = refresh_counts.get(refresh_key, 0) + 1
-                if refresh_counts[refresh_key] > _MAX_AUTH_REFRESH_ATTEMPTS:
-                    _ra().logger.warning(
-                        "Credential auth failure persists after %s refreshes for "
-                        "pool entry %s — treating as unrecoverable and allowing "
-                        "fallback to activate.",
-                        refresh_counts[refresh_key] - 1,
-                        refreshed_id,
-                    )
-                    return False, has_retried_429
            _ra().logger.info(f"Credential auth failure — refreshed pool entry {getattr(refreshed, 'id', '?')}")
            agent._swap_credential(refreshed)


🟡 Auth token refresh cap removed — infinite retry loop on dead OAuth credentials (bug)

The patch deleted _MAX_AUTH_REFRESH_ATTEMPTS = 2 and all refresh-count logic from recover_with_credential_pool() in agent/agent_runtime_helpers.py. This cap prevented infinite retry loops when a single-entry OAuth credential pool's token keeps being rejected after refresh — the cap would force fallback chain activation after 2 consecutive same-entry refreshes. Without it, try_refresh_current() succeeds (produces a fresh token) but the upstream keeps returning 401/403, creating an unbounded retry loop that never reaches the cross-provider fallback.

💡 Suggestion: Restore the _MAX_AUTH_REFRESH_ATTEMPTS constant and the per-refresh counting logic that forces fallback activation after consecutive same-entry refreshes.

📋 Prompt for AI Agents

In agent/agent_runtime_helpers.py, after line 776 (refreshed = pool.try_refresh_current()), add the refresh-count guard: track agent._auth_pool_refresh_counts keyed by (agent.provider, getattr(refreshed, 'id', None)), increment on each refresh, and return (False, has_retried_429) when the count exceeds 2.

tenki-reviewer · 2026-07-01T16:43:12Z

            elif base_url_host_matches(effective_base, "api.routermint.com"):
                client_kwargs["default_headers"] = _ra()._routermint_headers()
-            elif base_url_host_matches(effective_base, "githubcopilot.com"):
+            elif base_url_host_matches(effective_base, "api.githubcopilot.com"):


🟡 GitHub Copilot endpoint detection narrowed — enterprise subdomains not detected for header injection (bug)

The patch changed hostname matching for GitHub Copilot from githubcopilot.com to api.githubcopilot.com in agent_init.py:791, run_agent.py, and agent_runtime_helpers.py. Since base_url_host_matches does suffix-based matching, githubcopilot.com matches all subdomains including api.enterprise.githubcopilot.com and api.business.githubcopilot.com, but api.githubcopilot.com only matches the api subdomain. Enterprise Copilot users will not get copilot-specific headers. Additionally, many other files (auxiliary_client.py, chat_completion_helpers.py) still use the original githubcopilot.com pattern, creating divergent behavior across code paths.

💡 Suggestion: Use githubcopilot.com (without the api. prefix) throughout, or add explicit enterprise subdomain checks alongside api.githubcopilot.com.

📋 Prompt for AI Agents

In agent_init.py line 791, run_agent.py, and agent_runtime_helpers.py create_openai_client, revert the api.githubcopilot.com strings back to githubcopilot.com. In run_agent.py _is_github_copilot_url, restore: return hostname == "api.githubcopilot.com" or hostname.endswith(".githubcopilot.com").

tenki-reviewer · 2026-07-01T16:43:12Z

-                # stored under the wrong key ``"export API_KEY"`` (#6659).
-                if line.startswith('export '):
-                    line = line[7:]
                key, _, value = line.partition('=')


🟡 .env parsing regression: export-prefix stripping and structured-value detection removed (bug)

Two changes in hermes_cli/config.py break .env file parsing: (1) The load_env() function no longer strips the bash-compatible export prefix from lines like export API_KEY=..., causing such keys to be stored under the wrong name export API_KEY instead of API_KEY (previously fixed in issue NousResearch#6659). (2) The _sanitize_env_lines function removed the _looks_like_structured_value guards that prevented incorrect splitting of concatenated .env lines containing URLs or query strings with embedded = signs. This can cause secret truncation and fabrication of bogus entries.

💡 Suggestion: Restore the export prefix stripping in load_env() and the _looks_like_structured_value guard in _sanitize_env_lines.

📋 Prompt for AI Agents

In hermes_cli/config.py: (1) In load_env(), re-add the check if line.startswith('export '): line = line[7:] before parsing. (2) In _sanitize_env_lines, re-add the _looks_like_structured_value guard and split_into_entries logic to prevent incorrect splitting of URL-containing values.

tenki-reviewer · 2026-07-01T16:43:12Z

    # Uses the module-level `OpenAI` name, resolved lazily on first
    # access via __getattr__ below. Tests patch via `run_agent.OpenAI`.
    client = _ra().OpenAI(**client_kwargs)


🟡 max_retries=0 removed from create_openai_client — SDK-level retries reintroduced causing double-retry and ignored Retry-After (bug)

In agent/agent_runtime_helpers.py, the create_openai_client function no longer sets client_kwargs.setdefault('max_retries', 0). The removed comment explicitly warned: 'Delegate all rate-limit / 5xx retry to hermes's outer conversation loop, which honors Retry-After and applies adaptive/jittered backoff. The OpenAI SDK default (max_retries=2) uses its own 1-2s backoff that ignores Retry-After and double-retries inside our loop — the same deadlock the Anthropic clients hit (NousResearch#26293).' This change means the OpenAI SDK will now retry on its own (up to 2 times) in addition to Hermes' outer conversation loop retries.

💡 Suggestion: Restore the client_kwargs.setdefault('max_retries', 0) line in create_openai_client to prevent double-retry behavior and ensure Hermes' conversation loop is the sole retry mechanism.

📋 Prompt for AI Agents

In agent/agent_runtime_helpers.py, inside create_openai_client, re-add client_kwargs.setdefault('max_retries', 0) before the OpenAI client is constructed (before line 1433).

fix(agent): honor custom CA certs for custom_providers HTTPS endpoints

f8f4f29

tenki-reviewer Bot reviewed Jul 1, 2026

View reviewed changes

Navigation Menu

fix(agent): honor custom CA certs for custom_providers HTTPS endpoints - #337

fix(agent): honor custom CA certs for custom_providers HTTPS endpoints#337
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56393

hashbender commented Jul 1, 2026

Uh oh!

tenki-reviewer Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

tenki-reviewer Bot left a comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		def save_config(config: Dict[str, Any]):
		"""Save configuration to ~/.hermes/config.yaml."""

Conversation

hashbender commented Jul 1, 2026

Summary

Test plan

Uh oh!

tenki-reviewer Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tenki-reviewer Bot left a comment

Choose a reason for hiding this comment

PR #337 — Critical Review: TLS/Config/Agent Runtime Refactor

Critical Issues (must fix before merge)

High-Severity Regressions

Medium Issues

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

tenki-reviewer Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tenki-reviewer Bot commented Jul 1, 2026 •

edited

Loading